Skip to content

Feature: refactor DTLS to merge it into tls_openssl.c #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 270 commits into from

Conversation

JackLau1222
Copy link
Collaborator

@JackLau1222 JackLau1222 commented May 12, 2025

This patch is based on #1 but do meangful refactor work. The below is details

  1. Registers ff_dtls_protocol and links it automatically when the WHIP muxer is enabled.
  2. Merges TLS & DTLS implementation, shared BIO callbacks, read, write, print_ssl_error, openssl_init_ca_key_cert, init_bio_method function and shared same data structure
  3. enable re-use udp from demuxer or muxer in DTLS.
  4. delete some unnessary value or function like arq_pkts, state_callback and so on...

You can follow the usage to test this patch.

This PR has been squashed into a single commit on the patch/whip/v2 branch for easier submission to the FFmpeg community for code review.

This PR was merged to FFmpeg, see FFmpeg@167e343

@winlinvip winlinvip requested a review from Copilot May 12, 2025 01:37
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the DTLS implementation by merging it with the existing TLS code, introducing shared data structures and functions and integrating DTLS support into the WHIP muxer.

  • Introduces new DTLS fields and enum in the TLSShared structure in tls.h.
  • Modifies connection and protocol selection logic in tls.c to differentiate between TCP and UDP usage for TLS/DTLS.
  • Updates related modules (srtp, protocols, http, avio, allformats, Makefile, and documentation) to support the merged DTLS/TLS implementation.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libavformat/tls.h Added DTLS-specific fields, enum DTLSState, and new AV options.
libavformat/tls.c Updated connection logic to select UDP for DTLS and added functions for DTLS.
libavformat/srtp.h Changed SRTPContext to a typedef.
libavformat/protocols.c Declared the new ff_dtls_protocol.
libavformat/http.h & http.c Added function(s) for new HTTP location retrieval.
libavformat/avio.c Extended warning message to check for dtls.
libavformat/allformats.c Adjusted muxer declarations to include WHIP muxer support.
libavformat/Makefile Integrated new object files for the WHIP muxer build.
doc/muxers.texi Provided documentation for the new WHIP muxer.
configure Updated configuration to support the whip_muxer dependency on openssl.

Copy link
Member

@winlinvip winlinvip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this PR, and it works as expected. Thanks for your good work.

@JackLau1222 JackLau1222 force-pushed the feature/whip-dtls branch 2 times, most recently from cb68e9b to a9a670c Compare May 17, 2025 00:37
QSXW added 3 commits May 17, 2025 09:22
prepare for adaptive color transform

Signed-off-by: Wu Jianhua <[email protected]>
passed files:
    ACT_A_Kwai_3.bit
    ACT_B_Kwai_3.bit

Signed-off-by: Wu Jianhua <[email protected]>
fhvwy and others added 20 commits May 17, 2025 11:23
Buffers are allocated inside some metadata types, so we must ensure
that the object is visible to the free function before a parse failure.

Found by libFuzzer.
Also return an better error code if it is set numerically.
(This option was added in 2862b63
when an AVCodecContext generic option was moved to
a codec private one without realizing that not every
generic one is valid for every encoder.)

Reviewed-by: Jan Ekström <[email protected]>
Signed-off-by: Andreas Rheinhardt <[email protected]>
Fixes remaining \r\n is ASS header after 57c5450.

Signed-off-by: Kacper Michajłow <[email protected]>
Signed-off-by: Marton Balint <[email protected]>
Fixes remaining \r\n is ASS header after 57c5450.

Signed-off-by: Kacper Michajłow <[email protected]>
Signed-off-by: Marton Balint <[email protected]>
Fixes remaining \r\n is ASS header after 57c5450.

Fixes AVERROR_BUG error during init as this decoder expected `\r\n` in
default ASS header. strstr(..., "\r\n[Events]\r\n") failed after changes
in 57c5450.

Fixes ticket #11545.

Fixes: 57c5450
Signed-off-by: Kacper Michajłow <[email protected]>
Signed-off-by: Marton Balint <[email protected]>
The old limits were a bit too tightly clustered around 1.0. Make the
value range much more generous, and also introduce a new highlight
for speedups above 10.0 (order of magnitude improvement).
So we can move pass-adding business logic outside of graph.c.
The current loop only works if the input and output have the same number
of planes. However, with the new scaling logic, we can also optimize into a
noop the case where the input has extra unneeded planes.

For the memcpy fallback to work in these cases we have to instead check if
the *output* pointer is set, rather than the input pointer.
This is more consistent with the rest of the newly added code, which
universally switched to using bools for boolean values.
This should hopefully serve as a better introduction to my new swscale
redesign than hunting down random commit message monologues.
Avoids writing an empty json blob in setup error cases.
I have several .ts captures where video and audio codec changes even
though the PMT version does not change and the PIDs stay the same.
This happens during transition to/from slate (mpeg2 video and audio)
to network broadcast (hevc video and eac3 audio in private PES).

I've updated fate ts-demux expected results.
Silences warnings like

filters.texi:256: warning: no htmlxref.cnf entry found for `ffmpeg-utils'

Signed-off-by: James Almer <[email protected]>
No effect as is, but without this change, new additions to FATE_PIXFMT_16-*
will not work.

Signed-off-by: James Almer <[email protected]>
mkver and others added 28 commits June 2, 2025 00:47
Reviewed-by: softworkz . <[email protected]>
Signed-off-by: Andreas Rheinhardt <[email protected]>
Forgotten after d76b0c4.

Reviewed-by: softworkz . <[email protected]>
Signed-off-by: Andreas Rheinhardt <[email protected]>
The AVIOContext will be automatically flushed upon closure.

Reviewed-by: softworkz . <[email protected]>
Signed-off-by: Andreas Rheinhardt <[email protected]>
Reviewed-by: Martin Storsjö <[email protected]>
Signed-off-by: Andreas Rheinhardt <[email protected]>
The code to initialize the ungrouped bap mantissa tables
(bap 3 or 5) takes more bytes of .text than the tables itself;
they have therefore been hardcoded.

For GCC (14, -O3, albeit in an av_cold function), the initialization
code takes 99B each for the fixed and floating point decoders
(the code is currently duplicated), whereas the hardcoded tables
only take 96B. For Clang 19 it were 374B each (I don't now what
Clang was doing there).

Signed-off-by: Andreas Rheinhardt <[email protected]>
(I don't know why the encoder only uses eight of the nine values.)

Signed-off-by: Andreas Rheinhardt <[email protected]>
Signed-off-by: Andreas Rheinhardt <[email protected]>
avg_pixels_tab[0][0] does the same as avg_no_rnd_pixels_tab[0].

Signed-off-by: Andreas Rheinhardt <[email protected]>
Since a51279b,
hpeldsp_rnd_template.c was only included once and
one of the two functions in rnd_template.c was
only enabled once.

Signed-off-by: Andreas Rheinhardt <[email protected]>
Signed-off-by: Emma Worley <[email protected]>
Signed-off-by: Michael Niedermayer <[email protected]>
Adds a generic hash table with the DXV encoder as an initial use case.

Signed-off-by: Emma Worley <[email protected]>
Offers a modest performance gain due to the switch from naive linear
probling to robin hood.

Signed-off-by: Emma Worley <[email protected]>
Improves compatibility with Resolume products by adding an additional
hashtable for DXT color+LUT combinations, and padding the DXT texture
dimensions to the next largest multiple of 16. Produces identical
packets to Resolume Alley in manual tests.

Signed-off-by: Emma Worley <[email protected]>
0. Version 1.
1. The WHIP muxer has been renamed and refined, with improved logging context and error messages for SSL, DTLS, and RTC.
2. Magic numbers have been replaced with macros and extracted to functions, and log levels have been altered for better clarity.
3. DTLS curve list has been updated, and SRTP profile names have been refined for FFmpeg and OpenSSL.
4. ICE STUN magic number has been refined, and RTP payload types have been updated based on Chrome's definition.
5. Fixed frame size has been refined to rtc->audio_par->frame_size, and h264_mp4toannexb is now used to convert MP4/ISOM to annexb.
6. OPUS timestamp issue has been addressed, and marker setting has been corrected after utilizing BSF.
7. DTLS handshake and ICE handling have been optimized for improved performance, with a single handshake timeout and server role to prevent ARQ.
8. DTLS BIO callback has been implemented for packet fragmentation, and MTU settings have been refined using SSL_set_mtu and DTLS_set_link_mtu.
9. Consolidated ICE request/response handling and DTLS handshake into a single function, and fixed OpenSSL build errors to work with Pion.

------

Co-authored-by: winlin <[email protected]>
Co-authored-by: yangrtc <[email protected]>
Co-authored-by: cloudwebrtc <[email protected]>
Co-authored-by: Haibo Chen <[email protected]>
Signed-off-by: Steven Liu <[email protected]>
Signed-off-by: Jack Lau <[email protected]>

Abstract dtls as ffmpeg protocol

Since i make the dtls as a ffmpeg protocol, we need init it use ffurl_oepn after ICE binding. But before that, SDP need the fingerprint of cert
So i implement these gen or read in separated function and can be called by whip because we need obtain fingerprint for sdp exchange.
So we need pass the string of key and cert into dtls from whip.

debug: set info callback for tls_openssl

dtls: refactor the logic of bio

make the dtls use custom bio rather than callback to match the tls implementation

dtls: add option that re-use udp from demuxer/muxer

integrate print_ssl_error, read, write as same as implementation in tls

remove openssl_dtls_state_trace, opessl_ssl_get_error.

implemente dtls_handshake function.

pass patchcheck

fix tailing whitespace, missing av_cold, x==0 can be simplified, forget use av_log, use snprintf instead of sprintf, add braces for if else (Don’t wrap single-line blocks in braces. Use braces only if there is an accompanying else statement.)

Signed-of-by: Jack Lau <[email protected]>
handshake spells error

Co-authored-by: Copilot <[email protected]>
improve the api name like url_read_all -> ff_url_read_all.

fix av_log in tls_openssl.

use thread-safe function openssl_get_error instead of ERR_error_string(ERR_get_error(), NULL) in part of tls_openssl. In other case, use the ERR_error_string(ERR_get_error(), NULL) when there is no TLSContext.
This patch make the whip muxer depends dtls protocol,
and dtls protocol depends openssl for now.

Signed-off-by: Jack Lau <[email protected]>
@winlinvip winlinvip closed this Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.